-
Notifications
You must be signed in to change notification settings - Fork 333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SatlasPretrain: ResNet50/152 and Swin_V2_T Weights #2038
SatlasPretrain: ResNet50/152 and Swin_V2_T Weights #2038
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to apply all the same changes I requested on #1884
Also, CI passes because we only use fake model weights, but using the real model weights result in test failures:
> pytest -m slow tests/models/test_resnet.py
...
E RuntimeError: Attempting to deserialize object on a CUDA device but torch.cuda.is_available() is False. If you are running on a CPU-only machine, please use torch.load with map_location=torch.device('cpu') to map your storages to the CPU.
num_channels = weights.meta['in_chans'] | ||
out_channels = model.features[0][0].out_channels | ||
# https://github.com/allenai/satlaspretrain_models/blob/main/satlaspretrain_models/models/backbones.py#L27 | ||
model.features[0][0] = torch.nn.Conv2d( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was completely broken for MS models until now.
@@ -272,3 +272,54 @@ def apply_transform( | |||
out = rearrange(out, 'b t c h w -> (b t) c h w') | |||
|
|||
return out | |||
|
|||
|
|||
class _Clamp(K.IntensityAugmentationBase2D): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johnnv1 is this something Kornia would be interested in? If so I can contribute it when I have a chance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so, not sure why it will be a augmentation. But i think it's fine to add to kornia if have uses for it. Just to make sure, @shijianjian we don't have it, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
related to kornia/kornia#941
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any published performance metrics for all weights? Would love to add these to the weights tables. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will wait a bit more for further review but it sounds like everything is in order now.
Looking into if we have performance metrics that could fit into the torchgeo tables. Everything else looks good to me. Thank you both so much for making this PR! |
It doesn't have to be the same benchmark datasets we used, it could be different benchmarks.
I'm planning on releasing TorchGeo 0.6.0 by the end of this month, so hopefully he can review by Monday. |
Going to merge so we can finalize the release soon. @favyen2 can review later if he has time. |
This PR adds SatlasPretrain ResNet50 weights as the Enums.
@piperwolters @favyen2